LF-4564 unnest tasks from animals #3582
Conversation
antsgar
requested review from
Duncan-Brain and
kathyavini
and removed request for
a team
|
I am currently seeing that the created in error option is not shown for all animals.... looking into it. I like this solution! I think Sayakas concern was with efficiency of querying tasks. My main concern was nesting, and I didn't originally know there was a backend check going in. So that was a bug-concern and probably influenced my thoughts on recopying the existing pattern, but that is in now with #3546! So last thing was concurrency and IMO who cares about that as long as we check on the backend before writing to db! haha We could consider refetching tasks + invalidating animal tags to cover that on deletion/removal failure for the future... but I don't need it now. But yah looking into the bug I am seeing. |
|
@Duncan-Brain thank you so much for testing thoroughly!! I didn't have much time honestly and I checked for an animal that did have finalized tasks and figured it was working correctly 🤦🏻♀️ |
| } | ||
| const finalizedTasks = getFinalizedTasks(); | ||
|
|
||
| return selectedInventoryIds.some((id) => |
There was a problem hiding this comment.
@antsgar I think this would do it -- but its not pretty:
return selectedInventoryIds.some((animalOrBatchId) => {
const {id, kind} = parseInventoryId(animalOrBatchId);
return finalizedTasks.filter(
({ animals, animal_batches }: { animals: Animal[]; animal_batches: AnimalBatch[] }) => {
const animalIds = animals.map(({ id }) => `${id}`);
const batchIds = animal_batches.map(({ id }) => `${id}`);
return kind === AnimalOrBatchKeys.ANIMAL ? animalIds.includes(`${id}`) : batchIds.includes(`${id}`);
},
).length > 0;
}
);
There was a problem hiding this comment.
Made a PR for it if helpful -- .. removed redundant > 0
There was a problem hiding this comment.
Fixes both the type error crash and the deletion-ablity bug that would have needed tag invalidation so I approve 👍
Just wanted to make sure @SayakaOno had a chance to take a look, too.
I do still agree in general that we want to move heavy filtering to the backend, and I would be in favour of revisiting the other solution if this looks like it's making the app slow when tasks pile up!
This lines up with my understanding of optimisation 😝 : https://wiki.c2.com/?RulesOfOptimization |
Description
This was initially suggested here #3554 (comment)
Updates logic in the UI that verifies if animals/batches have finalized tasks to check the tasks themselves instead of relying on tasks being nested within animals.
Eliminates nested tasks from animals in API.
Removes need to invalidate animal/batch tags on task creation/deletion.
Jira link:
https://lite-farm.atlassian.net/browse/LF-4564
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: